Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

entproto: support enum values with special characters #487

Merged
merged 2 commits into from
May 9, 2023

Conversation

jamslinger
Copy link
Contributor

This fixes: ent/ent#3498

Copy link
Collaborator

@rotemtam rotemtam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this . Please handle the case of two enums that normalize to a identical string with an informative error?

@jamslinger jamslinger force-pushed the enum-value-normalization branch from cb328f6 to 18fd280 Compare April 21, 2023 10:25
@jamslinger
Copy link
Contributor Author

I like this . Please handle the case of two enums that normalize to a identical string with an informative error?

Thanks for your feedback, that's a good point. I assume you mean two values within a single enum right? I updated the PR accordingly and also added a test.

I also factored out a entproto.NormalizeEnumIdentifier function so that we don't rely on the same normalization logic being implemented in multiple different places. It is still being defined twice though, in entproto.NormalizeEnumIdentifier as well as in the enum template.

I also have a question: We are not being consistent in error messages with strings vs. quoted strings and I assume we are stuck with this for backwards compatibility?

@jamslinger jamslinger requested a review from rotemtam May 3, 2023 08:10
Copy link
Collaborator

@rotemtam rotemtam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @jamslinger !

@rotemtam
Copy link
Collaborator

rotemtam commented May 5, 2023

@jamslinger CI is red, can you check ?

@jamslinger jamslinger force-pushed the enum-value-normalization branch from 18fd280 to d53d12b Compare May 8, 2023 08:21
@jamslinger
Copy link
Contributor Author

I did a go mod tidy and it should be fixed now.

@jamslinger jamslinger requested a review from rotemtam May 8, 2023 08:25
@rotemtam rotemtam merged commit 589b2d1 into ent:master May 9, 2023
@rotemtam
Copy link
Collaborator

rotemtam commented May 9, 2023

Thanks for the contribution @jamslinger !

@jamslinger
Copy link
Contributor Author

Thanks for the support @rotemtam :)

@jamslinger jamslinger deleted the enum-value-normalization branch May 24, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

entproto: enum values with special characters are not supported
2 participants